-
Notifications
You must be signed in to change notification settings - Fork 35
Test sendmany
#344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test sendmany
#344
Conversation
39a0deb
to
5e09bbd
Compare
integration_test/tests/wallet.rs
Outdated
let model: Result<mtype::SendMany, _> = json.into_model(); | ||
let txid = model.unwrap().0; | ||
|
||
assert_eq!(txid.to_string().len(), 64); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this line aiming to test mate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just want to check that txid holds a txid, but I don't know in advance what it is so I just check it is the correct length.
There were other tests that I thought were working because the field I tested in the returned RPC was not empty, but then looking further the field was populated by an error message.
Do you have a better suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we've called into_model
the txid should have been parsed from the string so an error message or any other junk will be caught there - I would expect it to be anyways, otherwise something is very wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good apart from the txid thing.
Reviewed 5e09bbd |
`sendmany` has a new verbose return type in v18. The client macro sends the amount of sats but the RPC takes BTC. Add the verbose struct to v18 and model. Update the client macro. Add a verbose client macro, test and update the types table.
5e09bbd
to
175bafd
Compare
Removed the redundant txid check and rebased on master. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 175bafd
sendmany
is implemented for v17 but untested and it has a new verbose return type in v18.The client macro sends the amount in sats but the RPC takes BTC.